Skip to content

Conversation

@gunpal5
Copy link
Collaborator

@gunpal5 gunpal5 commented Mar 10, 2025

  • Added a dedicated attribute FunctionToolAttribute for conversion of methods to Tools e.g.
[FunctionTool]
public Task<Weather> GetCurrentWeatherAsync(string location, Unit unit = Unit.Celsius, CancellationToken cancellationToken = default)
{
    return Task.FromResult(new Weather
    {
        Location = location,
        Temperature = 22.0,
        Unit = unit,
        Description = "Sunny",
    });
}

var tools = new Tools([GetCurrentWeatherAsync])

//Access list of CSharpToJsonSchema.Tool
var myTools = tools.AvailableTools

//Implicit Conversion to list of CSharpToJsonSchema.Tool
List<Tool> myTools = tools
  • Added Optional Parameter GoogleFunctionTool into GenerateJsonSchema and FunctionTool to generate AsGoogleFunctionTool() methods for seemless integration with Google_GenerativeAI SDK

Summary by CodeRabbit

  • Documentation

    • Expanded the features section with a new "Methods" area that includes clear examples on defining functions using the new attribute for method-based tools.
  • New Features

    • Enhanced function tool support, including integration for advanced tool operations (synchronous and asynchronous). These updates improve how functions are defined and utilized for enhanced developer productivity.

@coderabbitai
Copy link

coderabbitai bot commented Mar 10, 2025

Walkthrough

The changes update documentation and enhance source generation features. The README now includes an expanded features section and a new "Methods" example. Several generator classes have been modified to support function tools and Google Function Tool extensions, including new methods, altered signatures, and additional helper functions. Internal refactoring of method calls and adjustments in attribute parsing, namespace handling, and incremental data collection have been made. New tests for function tools have also been added, covering both synchronous and asynchronous scenarios.

Changes

File(s) Change Summary
README.md Enhanced features section; added new "Methods" section with a complete code snippet for GetCurrentWeatherAsync.
Conversion/SymbolGenerator.cs
Conversion/ToModels.cs
Added GenerateToolJsonSerializerContext in SymbolGenerator; updated PrepareData and PrepareMethodData to support a new boolean flag and added GetCommonRootNamespace.
JsonGen/JsonSourceGenerator.Parser.cs
JsonGen/JsonSourceGenerator.Roslyn4.0.cs
Changed ParseContextGenerationSpec signature to use an ImmutableArray; added constant FunctionToolAttributeFullName and new method InitializeForFunctionTools.
JsonSchemaGenerator.cs Modified ProcessMethods to invoke new tool handlers (AsFunctionCalls, AsGoogleFunctionToolsForMethods, AsGoogleFunctionToolsForInterface); updated method signature for preparing method data.
Models/InterfaceData.cs
FunctionToolAttribute.cs
GenerateJsonSchemaAttribute.cs
Added property GoogleFunctionTool in each to flag Google Function Tool support.
Sources.Method.Calls.cs
Sources.Method.GoogleFunctionTools.cs
Sources.Method.Tools.cs
Sources.Tools.cs
Refactored Sources class with new names and signatures; added methods that generate Google Function Tool related code and client implementation; removed redundant tool client generation methods.
Steps.cs Updated SelectManyAllAttributesOfCurrentMethodSyntax to return an ImmutableArray wrapped in an IncrementalValueProvider.
Test Files:
JsonSerializationTests.cs
MethodFunctionTools_Tests.cs
AotTests/Services/MethodFunctionTools.cs
IntegrationTests/MethodFunctionTools.cs
SnapshotTests/SnapshotTests.cs
Added new test classes for function tool methods; renamed and added several tool methods; applied formatting adjustments and included a commented-out snapshot test.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Tools
    participant DelegateLookup
    participant FunctionToolMethod

    Client->>Tools: CallAsync("FunctionName", json)
    Tools->>DelegateLookup: Lookup delegate for function
    DelegateLookup-->>Tools: Return delegate
    Tools->>FunctionToolMethod: Invoke function with json
    FunctionToolMethod-->>Tools: Return result
    Tools-->>Client: Send back result
Loading

Poem

I’m a rabbit in the code field, hopping through each line,
New methods and tools now in place, oh how they shine!
Function tools spring to life with every clever hop,
Google features guide my leaps—there’s no way to stop.
In CodeRabbit land, our code’s a joyful song,
Every change a carrot crunch, making the journey strong!

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ gunpal5
❌ Gunpal Jain


Gunpal Jain seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🔭 Outside diff range comments (1)
src/libs/CSharpToJsonSchema.Generators/Sources.Method.Calls.cs (1)

30-63: 🛠️ Refactor suggestion

Possible key lookup risk in AsCalls().

Delegates["{method.Name}"] is directly accessed with no checks, potentially causing a KeyNotFoundException. Consider adding a safety check or fallback.

🧹 Nitpick comments (24)
src/libs/CSharpToJsonSchema.Generators/Conversion/SymbolGenerator.cs (1)

99-151: New method for generating JSON serializer context for tools.

The GenerateToolJsonSerializerContext method creates a class declaration for ToolsJsonSerializerContext marked as both public and partial. This aligns with the PR objective of supporting function tools and Google Function Tool integration.

However, there's significant code duplication between this method and the existing GenerateParameterBasedClassSymbol method (lines 16-98). Consider refactoring to extract common code for creating classes, namespaces, and compilation units into a separate helper method.

+ private static INamedTypeSymbol? GenerateTypeSymbol(
+     string rootNamespace,
+     ClassDeclarationSyntax classDecl,
+     Compilation originalCompilation)
+ {
+     var namespaceName = rootNamespace;
+     var ns = SyntaxFactory.NamespaceDeclaration(SyntaxFactory.IdentifierName(namespaceName))
+         .AddMembers(classDecl);
+ 
+     var compilationUnit = SyntaxFactory.CompilationUnit()
+         .AddMembers(ns)
+         .NormalizeWhitespace();
+     
+     var parseOptions = CSharpParseOptions.Default.WithLanguageVersion(originalCompilation.GetLanguageVersion() ?? LanguageVersion.Default);
+     var syntaxTree = CSharpSyntaxTree.Create(compilationUnit, parseOptions);
+ 
+     var compilation = originalCompilation
+         .AddSyntaxTrees(syntaxTree);
+     
+     var semanticModel = compilation.GetSemanticModel(syntaxTree);
+ 
+     var classNode = syntaxTree.GetRoot().DescendantNodes()
+         .OfType<ClassDeclarationSyntax>()
+         .FirstOrDefault();
+ 
+     if (classNode == null) return null;
+ 
+     return semanticModel.GetDeclaredSymbol(classNode);
+ }

Then update both methods to use this helper.

src/libs/CSharpToJsonSchema.Generators/Sources.Tools.cs (1)

94-94: Whitespace adjustment.

This appears to be part of a larger restructuring. Based on the AI summary, the GenerateFunctionToolClientImplementation and GetInputsTypes methods have been removed from this file and a new file (Sources.Method.Tools.cs) was created to handle function tool client implementations.

This restructuring aligns with the PR objective to support individual methods as tools. Consider adding a comment here referencing the new implementation location for better code discoverability.

-   
+   // Function tool client implementation moved to Sources.Method.Tools.cs
src/libs/CSharpToJsonSchema.Generators/JsonGen/JsonSourceGenerator.Roslyn4.0.cs (2)

68-112: Clean up code implementation and remove unnecessary comments.

The InitializeForFunctionTools method correctly mirrors the structure of the existing Initialize2 method, adapting it to work with methods instead of interfaces. However, there are several issues to address:

  1. There's an empty line with whitespace on line 91
  2. There's a commented-out line on line 97 that should be removed
  3. There's a commented-out chained method call on line 105
                .Collect().Select(static (tuple,CancellationToken) =>
                {
-                        
+                        
                        var known = tuple.FirstOrDefault();
                        var knowntype = known.Right;
                        if (knowntype == null && !tuple.Any())
                        {
                            return (null, ImmutableEquatableArray<DiagnosticInfo>.Empty);
-                            //knowntype =new KnownTypeSymbols(tuple.First().Left.SemanticModel.Compilation);
                        }
                        Parser parser = new(knowntype);
                        Model.ContextGenerationSpec? contextGenerationSpec =
                            parser.ParseContextGenerationSpec(tuple,CancellationToken);
                        ImmutableEquatableArray<DiagnosticInfo> diagnostics =
                            parser.Diagnostics.ToImmutableEquatableArray();
                        return (contextGenerationSpec, diagnostics);
-                    })//.Select(sx=>(sx.contextGenerationSpec,sx.contextGenerationSpec))
+                    })

94-98: Review conditional logic for clarity and robustness.

The condition knowntype == null && !tuple.Any() is checking two things, but they seem potentially contradictory. If tuple is empty (!tuple.Any()), then knowntype would always be null because known would be default. Consider simplifying this logic.

                        var known = tuple.FirstOrDefault();
                        var knowntype = known.Right;
-                        if (knowntype == null && !tuple.Any())
+                        if (!tuple.Any())
                        {
                            return (null, ImmutableEquatableArray<DiagnosticInfo>.Empty);
                        }
src/libs/CSharpToJsonSchema.Generators/Sources.Method.Tools.cs (3)

23-24: Remove commented-out code.

The commented lines should be removed as they're not providing value and may cause confusion about the current implementation.

-        //var methodName = @interface.Methods.First().Name;
-        //var funcsToAdd = @interface.Methods.Select(GetInputsTypes).Distinct().ToList();

47-53: Enhance error handling for function tool registration.

When registering tools, the code correctly checks for duplicates (line 51), but the error handling for duplicate names is only implemented at delegate registration (lines 67-69). Consider adding a more descriptive error message here as well.

            tool = new global::CSharpToJsonSchema.Tool
            {{
                Name = ""{method.Name}"",
                Description = ""{method.Description}"",
                Strict = {(method.IsStrict ? "true" : "false")},
                Parameters = global::CSharpToJsonSchema.SchemaBuilder.ConvertToSchema(global::{@interface.Namespace}.{extensionsClassName}JsonSerializerContext.Default.{method.Name}Args,{"\""}{GetDictionaryString(method)}{"\""}),
            }};
-            if(!list.ContainsKey(tool.Name))
+            if(!list.ContainsKey(tool.Name))
                list.Add(""{method.Name}"",tool);
+            else
+                throw new global::System.Exception({"$\"Function {method.Name} is registered multiple times in the schema definition. Please ensure each function has a unique name.\""});

85-88: Simplify the AsTools method implementation.

The current implementation with the null-coalescing assignment operator can be simplified for better readability.

        public global::System.Collections.Generic.List<global::CSharpToJsonSchema.Tool> AsTools()
        {
-            return (global::System.Collections.Generic.List<global::CSharpToJsonSchema.Tool>) (this.AvailableTools??= new global::System.Collections.Generic.List<global::CSharpToJsonSchema.Tool>());
+            if (this.AvailableTools == null)
+            {
+                this.AvailableTools = new global::System.Collections.Generic.List<global::CSharpToJsonSchema.Tool>();
+            }
+            return (global::System.Collections.Generic.List<global::CSharpToJsonSchema.Tool>) this.AvailableTools;
        }
src/tests/CSharpToJsonSchema.AotTests/MethodFunctionTools_Tests.cs (1)

75-76: Remove trailing whitespace.

There is unnecessary whitespace at the end of the test method.

    public async Task Should_SampleFunctionTool_Static_Void()
    {
        var tools = new Tools([MethodFunctionTools.SampleFunctionTool_Static_Void]);
        await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_Static_Void), "{\"input\":\"test\"}");
-       
    }
src/libs/CSharpToJsonSchema.Generators/Steps.cs (1)

52-60: Improve code formatting for readability.

The formatting of the SelectMany lambda is inconsistent with the rest of the codebase. Consider standardizing the indentation and line breaks for better readability.

        var items = source
-            .SelectMany(static (context, _) => 
-                context.Attributes
-                .Select(x => 
-                (
-                    context.SemanticModel,
-                    AttributeData: x,
-                    ClassSyntax: (MethodDeclarationSyntax)context.TargetNode,
-                    ClassSymbol: (IMethodSymbol)context.TargetSymbol)
-                )).Collect();
+            .SelectMany(static (context, _) => context.Attributes
+                .Select(x => (
+                    context.SemanticModel,
+                    AttributeData: x,
+                    ClassSyntax: (MethodDeclarationSyntax)context.TargetNode,
+                    ClassSymbol: (IMethodSymbol)context.TargetSymbol)))
+            .Collect();
src/libs/CSharpToJsonSchema.Generators/Sources.Method.GoogleFunctionTools.cs (1)

38-39: Risk of substring out-of-range on interface names not prefixed with "I".
Using Substring(1) presumes that the interface name always starts with "I". This could trigger an exception for unusual interface names.

- var extensionsClassName = @interface.Name.Substring(startIndex: 1) + "Extensions";
+ var extensionsClassName = @interface.Name.StartsWith("I") && @interface.Name.Length > 1
+     ? @interface.Name.Substring(1) + "Extensions"
+     : @interface.Name + "Extensions";
src/libs/CSharpToJsonSchema.Generators/Conversion/ToModels.cs (2)

48-48: Property assignment in InterfaceData.
Setting GoogleFunctionTool as part of InterfaceData is straightforward. Consider documenting how it is used downstream for clarity.


53-98: Method-based interface data preparation.
The new PrepareMethodData method enumerates methods with their attributes. This is an effective approach for collecting data. Validate that skipping CancellationToken is intentional or re-apply the filter if needed.

src/libs/CSharpToJsonSchema.Generators/JsonGen/JsonSourceGenerator.Parser.cs (4)

229-278: Partial class checks in TryGetNestedTypeDeclarations.
Creating a default tools context declaration if the partial class or interface signature isn't found is a clever fallback. Just be sure partial definitions are consistently recognized across user code.


606-607: Appended "JsonSerializerContext" naming.
Appending "JsonSerializerContext" to the base name can be beneficial for clarity, though watch for possible naming collisions when the user code already ends with "Context."


791-832: Method-based attribute parsing.
Leveraging GenerateParameterBasedClassSymbol for method arguments is a flexible approach, especially for generating types on the fly. Ensure that any advanced generic parameters or constraints on these methods are handled properly.


966-1108: Parsing JsonSourceGenerationOptionsAttribute.
This extensive parsing logic thoroughly covers each property. Consider documenting how advanced scenarios (like custom policies or converters) might be extended or overridden if future requirements emerge.

src/libs/CSharpToJsonSchema.Generators/JsonSchemaGenerator.cs (3)

46-53: Consider consolidating repeated patterns.

Both AsFunctionCalls and AsGoogleFunctionToolsForMethods follow a similar pattern of SelectAndReportExceptions(...).AddSource(context). You might consider unifying these calls in one chain or function to reduce verbosity.


54-55: Ensure consistent initialization naming.

InitializeForFunctionTools(context) differs from Initialize2(context) used in ProcessInterfaces. Consider adopting consistent naming (e.g. InitializeForFunctionTools/InitializeForInterfaces) to reduce confusion for future maintainers.


90-95: Clarify the returned type.

PrepareMethodData returns an InterfaceData even though it processes method-specific data. Although functional, consider extracting a more precise type name (e.g., MethodData) if feasible, to improve clarity.

src/tests/CSharpToJsonSchema.AotTests/Services/MethodFunctionTools.cs (1)

8-55: Unused inputs and cancellation tokens.

All methods ignore the input parameter and do not utilize their CancellationToken. If this is a placeholder, it's fine; otherwise, consider using or removing these parameters to avoid confusion.

src/tests/CSharpToJsonSchema.IntegrationTests/MethodFunctionTools.cs (1)

9-56: Parameters are consistently unused.

All instance/static methods return hardcoded strings or complete tasks without leveraging input or CancellationToken. Confirm this is intentional. Removing or handling these parameters can improve clarity.

src/libs/CSharpToJsonSchema.Generators/Sources.Method.Calls.cs (3)

99-128: Use of dynamic may affect performance and type safety.

Relying on (dynamic)Delegates[...] could result in runtime issues if signatures or keys change. Consider strongly typed delegates for safer refactoring.


139-172: Async method invocation with dynamic.

Similar to the non-async approach, dynamic can introduce runtime errors. Consider stronger typing to ensure correctness.


189-198: Run-time dictionary lookups in CallAsync.

CallAsync presumes functionName always matches a valid dictionary entry. If user input can vary, consider robust error handling.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c867bd and 1ebee3f.

📒 Files selected for processing (19)
  • README.md (2 hunks)
  • src/libs/CSharpToJsonSchema.Generators/Conversion/SymbolGenerator.cs (1 hunks)
  • src/libs/CSharpToJsonSchema.Generators/Conversion/ToModels.cs (2 hunks)
  • src/libs/CSharpToJsonSchema.Generators/JsonGen/JsonSourceGenerator.Parser.cs (15 hunks)
  • src/libs/CSharpToJsonSchema.Generators/JsonGen/JsonSourceGenerator.Roslyn4.0.cs (2 hunks)
  • src/libs/CSharpToJsonSchema.Generators/JsonSchemaGenerator.cs (6 hunks)
  • src/libs/CSharpToJsonSchema.Generators/Models/InterfaceData.cs (1 hunks)
  • src/libs/CSharpToJsonSchema.Generators/Sources.Method.Calls.cs (6 hunks)
  • src/libs/CSharpToJsonSchema.Generators/Sources.Method.GoogleFunctionTools.cs (1 hunks)
  • src/libs/CSharpToJsonSchema.Generators/Sources.Method.Tools.cs (1 hunks)
  • src/libs/CSharpToJsonSchema.Generators/Sources.Tools.cs (1 hunks)
  • src/libs/CSharpToJsonSchema.Generators/Steps.cs (3 hunks)
  • src/libs/CSharpToJsonSchema/FunctionToolAttribute.cs (1 hunks)
  • src/libs/CSharpToJsonSchema/GenerateJsonSchemaAttribute.cs (1 hunks)
  • src/tests/CSharpToJsonSchema.AotTests/JsonSerializationTests.cs (3 hunks)
  • src/tests/CSharpToJsonSchema.AotTests/MethodFunctionTools_Tests.cs (1 hunks)
  • src/tests/CSharpToJsonSchema.AotTests/Services/MethodFunctionTools.cs (1 hunks)
  • src/tests/CSharpToJsonSchema.IntegrationTests/MethodFunctionTools.cs (1 hunks)
  • src/tests/CSharpToJsonSchema.SnapshotTests/SnapshotTests.cs (1 hunks)
🔇 Additional comments (32)
src/libs/CSharpToJsonSchema.Generators/Models/InterfaceData.cs (1)

6-6: LGTM: New property added correctly

The addition of the GoogleFunctionTool boolean property to the InterfaceData record struct aligns with the PR objective of supporting Google Function Tool extensions.

src/libs/CSharpToJsonSchema/FunctionToolAttribute.cs (1)

14-18: LGTM: Well-documented property addition

The new GoogleFunctionTool property is properly documented with a clear purpose - to generate Google Function Tools extensions for the Google GenerativeAI SDK. This aligns with the PR's stated objectives.

src/libs/CSharpToJsonSchema/GenerateJsonSchemaAttribute.cs (1)

15-19: LGTM: Consistent implementation

The addition of the GoogleFunctionTool property with appropriate documentation mirrors the implementation in the FunctionToolAttribute class, maintaining consistency across the codebase.

src/tests/CSharpToJsonSchema.SnapshotTests/SnapshotTests.cs (1)

6-10:

Details

✅ Verification successful

Is this test intentionally commented out?

This commented-out test method appears to be related to the new MethodFunctionTools functionality. Is it intentionally disabled, or should it be uncommented and included in the test suite?


🏁 Script executed:

#!/bin/bash
# Check if there are any actual tests for MethodFunctionTools functionality elsewhere
echo "Searching for MethodFunctionTools tests in the codebase..."
find . -name "*.cs" -type f | xargs grep -l "MethodFunctionTools" | grep -v "SnapshotTests.cs"

Length of output: 431


Test Coverage Confirmation – Intentional Exclusion of Snapshot Test

It appears that tests for the new MethodFunctionTools functionality are already implemented in other suites:

  • src/tests/CSharpToJsonSchema.AotTests/MethodFunctionTools_Tests.cs
  • src/tests/CSharpToJsonSchema.AotTests/Services/MethodFunctionTools.cs
  • src/tests/CSharpToJsonSchema.IntegrationTests/MethodFunctionTools.cs

Given this coverage, the commented-out test in SnapshotTests.cs (lines 6–10) looks to be intentionally disabled to prevent redundancy. Please confirm that this design decision aligns with your testing strategy.

README.md (2)

9-9: Clear documentation update for added feature.

The feature description now correctly includes the addition of defining functions through individual methods, which aligns with the new functionality being introduced in this PR.


61-84: Well-structured example showcasing the new FunctionTool attribute.

The new "Methods" section provides a clear and complete example of how to:

  1. Define a method with the [FunctionTool] attribute
  2. Create a Tools object with the method
  3. Access the available tools
  4. Utilize implicit conversion to a list of tools

This documentation effectively demonstrates the core functionality added by this PR.

src/tests/CSharpToJsonSchema.AotTests/JsonSerializationTests.cs (3)

33-35: Code formatting improvement.

The updated formatting of the JsonSerializer.Serialize call improves readability by properly aligning method parameters.


104-105: Code formatting improvement.

The updated formatting of the JsonSerializer.Deserialize call improves readability by properly aligning method parameters.


131-131: Whitespace correction.

Added whitespace at the end of the file for consistent formatting.

src/libs/CSharpToJsonSchema.Generators/Sources.Tools.cs (1)

64-92:

Details

✅ Verification successful

Consider updating method for Google Function Tool support.

The PR objective mentions adding an optional parameter GoogleFunctionTool to generate AsGoogleFunctionTool() methods. This existing method GenerateClientImplementation doesn't appear to have been updated to support this functionality.

Ensure this method handles the GoogleFunctionTool parameter if needed, or confirm that this functionality is implemented elsewhere (possibly in the new Sources.Method.Tools.cs file).


🏁 Script executed:

#!/bin/bash
# Search for GoogleFunctionTool implementation in the codebase
rg -A 5 "GoogleFunctionTool" --type cs

Length of output: 10792


Review Comment Update: Confirming Separate Handling of GoogleFunctionTool

The GenerateClientImplementation method remains unchanged because Google Function Tool support is implemented in a dedicated file—Sources.Method.GoogleFunctionTools.cs. The new methods (GenerateGoogleFunctionToolForMethods and GenerateGoogleFunctionToolForInterface) already handle scenarios based on the GoogleFunctionTool flag.

  • No modifications needed in GenerateClientImplementation.
  • Please verify that the implementations in Sources.Method.GoogleFunctionTools.cs meet the PR objectives for Google Function Tool support.
src/libs/CSharpToJsonSchema.Generators/JsonGen/JsonSourceGenerator.Roslyn4.0.cs (1)

24-24: LGTM! Good addition of constant for FunctionToolAttribute.

The constant for FunctionToolAttributeFullName is correctly defined, matching the namespace pattern used for the existing JsonSerializableAttributeFullName.

src/libs/CSharpToJsonSchema.Generators/Sources.Method.Tools.cs (3)

9-13: LGTM! Good entry validation.

The initial validation to check if there are any methods is important to avoid generating empty implementations.


57-70: LGTM! Good validation in AddTool method.

The AddTool method correctly initializes collections if needed, validates that the tool is registered in the schema, and prevents duplicate registrations.


93-99: LGTM! Well-structured helper method.

The GetInputsTypes method efficiently extracts parameter types and optionally includes the return type, which is useful for method signature uniqueness checks.

src/libs/CSharpToJsonSchema.Generators/Steps.cs (1)

47-48: LGTM! Updated return type to support collection.

The updated return type now correctly returns an IncrementalValueProvider<ImmutableArray<...>> instead of an IncrementalValuesProvider<...>, which aligns with the collection of results.

src/libs/CSharpToJsonSchema.Generators/Sources.Method.GoogleFunctionTools.cs (4)

10-11: Skip generating code if toolbox is not enabled or lacks methods.
This condition correctly avoids redundant code generation.


21-24: Implicit operator usage looks correct.
Allowing an implicit conversion to GenericFunctionTool is convenient for consumers. Ensure that this operator won’t cause unexpected conversions if there's another operator accepting the same type.


26-29: Utility method fosters clarity.
Returning GenericFunctionTool via AsGoogleFunctionTool() is intuitive. The method name clearly communicates its purpose.


36-37: Consistent condition check.
Similar to the first method, this prevents generating unnecessary code when GoogleFunctionTool is not enabled.

src/libs/CSharpToJsonSchema.Generators/Conversion/ToModels.cs (2)

19-20: Good approach to read named argument for GoogleFunctionTool.
Reading the GoogleFunctionTool argument into generateGoogleFunctionTool is succinct and clear.


99-132: Extracting common root namespace.
GetCommonRootNamespace is a neat solution to unify interfaces’ namespaces. However, if the namespace list is empty or all differ from the first segment, this might yield null. Make sure to handle these scenarios gracefully downstream.

src/libs/CSharpToJsonSchema.Generators/JsonGen/JsonSourceGenerator.Parser.cs (3)

102-128: Context generation early return logic.
Short-circuiting with return null when _compilationContainsCoreJsonTypes is false prevents invalid context creation. This is sensible. Just ensure you handle the null result in the caller.


129-228: Building rootSerializableTypes for context.
You gather root serializable types from each method. Confirm that merging them all into one list does not cause collisions if multiple methods define the same type with different considerations.


382-483: Alternate ParseContextGenerationSpec overload.
This overload appears consistent with the interface variant, but returns a context for a class declaration. Verify partial class constraints and fallback logic are mirrored from the interface version.

src/libs/CSharpToJsonSchema.Generators/JsonSchemaGenerator.cs (5)

31-31: Looks good.

Calling ProcessMethods(context) here helps consolidate method processing clearly after interface handling. No concerns.


73-75: Google function tools integration is consistent.

The call to generate Google function tools for interfaces aligns with the method-based approach. Looks good overall.


116-121: Implementation of AsFunctionCalls is straightforward.

No major issues. The method name is clear, and the usage in the codebase appears consistent.


123-128: Google function tools for methods.

This addition neatly integrates Google function tool generation for methods. Nicely structured.


130-135: Google function tools for interface.

Consistent with the method-based approach. No concerns.

src/libs/CSharpToJsonSchema.Generators/Sources.Method.Calls.cs (3)

66-95: Exception handling is sufficient for JSON deserialization.

The throw on deserialization failure is appropriate here. Just confirming that the internal usage is tested for invalid JSON scenarios.

Would you like a script to verify coverage for malformed JSON in unit tests?


130-136: Void method call logic matches the pattern.

The dictionary approach is consistent, but be mindful of potential missing keys or incorrect signatures at runtime.


176-187: Async void-equivalent approach returns an empty string.

Using Task<string> for a method that effectively returns void is valid, but be sure the design is intentional. The same caution about missing dictionary entries applies.

Comment on lines +25 to +33
[Fact]
public async Task Should_SampleFunctionTool_String()
{
var mf = new MethodFunctionTools();
var tools = new Tools([mf.SampleFunctionTool_String]);
var value = await tools.CallAsync(nameof(mf.SampleFunctionTool_String), "{\"input\":\"test\"}");

Assert.Equal(value, "\"Hello world from string return\"");
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix assertion parameter order.

Same issue with parameter order in the assertion.

    [Fact]
    public async Task Should_SampleFunctionTool_String()
    {
        var mf = new MethodFunctionTools();
        var tools = new Tools([mf.SampleFunctionTool_String]);
        var value = await tools.CallAsync(nameof(mf.SampleFunctionTool_String), "{\"input\":\"test\"}");

-        Assert.Equal(value, "\"Hello world from string return\"");
+        Assert.Equal("\"Hello world from string return\"", value);
    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[Fact]
public async Task Should_SampleFunctionTool_String()
{
var mf = new MethodFunctionTools();
var tools = new Tools([mf.SampleFunctionTool_String]);
var value = await tools.CallAsync(nameof(mf.SampleFunctionTool_String), "{\"input\":\"test\"}");
Assert.Equal(value, "\"Hello world from string return\"");
}
[Fact]
public async Task Should_SampleFunctionTool_String()
{
var mf = new MethodFunctionTools();
var tools = new Tools([mf.SampleFunctionTool_String]);
var value = await tools.CallAsync(nameof(mf.SampleFunctionTool_String), "{\"input\":\"test\"}");
Assert.Equal("\"Hello world from string return\"", value);
}

Comment on lines +62 to +68
[Fact]
public async Task Should_SampleFunctionTool_Static_String()
{
var tools = new Tools([MethodFunctionTools.SampleFunctionTool_Static_String]);
var value = await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_Static_String), "{\"input\":\"test\"}");
Assert.Equal(value, "\"Hello world from string return\"");
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix assertion parameter order.

Same issue with parameter order in the assertion.

    [Fact]
    public async Task Should_SampleFunctionTool_Static_String()
    {
        var tools = new Tools([MethodFunctionTools.SampleFunctionTool_Static_String]);
        var value = await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_Static_String), "{\"input\":\"test\"}");
-        Assert.Equal(value, "\"Hello world from string return\"");
+        Assert.Equal("\"Hello world from string return\"", value);
    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[Fact]
public async Task Should_SampleFunctionTool_Static_String()
{
var tools = new Tools([MethodFunctionTools.SampleFunctionTool_Static_String]);
var value = await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_Static_String), "{\"input\":\"test\"}");
Assert.Equal(value, "\"Hello world from string return\"");
}
[Fact]
public async Task Should_SampleFunctionTool_Static_String()
{
var tools = new Tools([MethodFunctionTools.SampleFunctionTool_Static_String]);
var value = await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_Static_String), "{\"input\":\"test\"}");
Assert.Equal("\"Hello world from string return\"", value);
}

Comment on lines +5 to +14
[Fact]
public async Task Should_SampleFunctionTool_StringAsync()
{
var mf = new MethodFunctionTools();
var tools = new Tools([mf.SampleFunctionTool_StringAsync]);
var value = await tools.CallAsync(nameof(mf.SampleFunctionTool_StringAsync), "{\"input\":\"test\"}");

Assert.Equal(value, "\"Hello world\"");

}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix assertion parameter order.

In xUnit, the convention is to place the expected value first and the actual value second in assertion methods. The current implementation reverses this order.

    [Fact]
    public async Task Should_SampleFunctionTool_StringAsync()
    {
        var mf = new MethodFunctionTools();
        var tools = new Tools([mf.SampleFunctionTool_StringAsync]);
        var value = await tools.CallAsync(nameof(mf.SampleFunctionTool_StringAsync), "{\"input\":\"test\"}");

-        Assert.Equal(value, "\"Hello world\"");
+        Assert.Equal("\"Hello world\"", value);
        
    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[Fact]
public async Task Should_SampleFunctionTool_StringAsync()
{
var mf = new MethodFunctionTools();
var tools = new Tools([mf.SampleFunctionTool_StringAsync]);
var value = await tools.CallAsync(nameof(mf.SampleFunctionTool_StringAsync), "{\"input\":\"test\"}");
Assert.Equal(value, "\"Hello world\"");
}
[Fact]
public async Task Should_SampleFunctionTool_StringAsync()
{
var mf = new MethodFunctionTools();
var tools = new Tools([mf.SampleFunctionTool_StringAsync]);
var value = await tools.CallAsync(nameof(mf.SampleFunctionTool_StringAsync), "{\"input\":\"test\"}");
Assert.Equal("\"Hello world\"", value);
}

Comment on lines +45 to +53
[Fact]
public async Task Should_SampleFunctionTool_Static_StringAsync()
{

var tools = new Tools([MethodFunctionTools.SampleFunctionTool_Static_StringAsync]);
var value = await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_Static_StringAsync), "{\"input\":\"test\"}");

Assert.Equal(value, "\"Hello world\"");
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix whitespace and assertion parameter order.

There's an unnecessary blank line and the assertion parameters are reversed.

    [Fact]
    public async Task Should_SampleFunctionTool_Static_StringAsync()
    {
-   
+       
        var tools = new Tools([MethodFunctionTools.SampleFunctionTool_Static_StringAsync]);
        var value = await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_Static_StringAsync), "{\"input\":\"test\"}");

-        Assert.Equal(value, "\"Hello world\"");
+        Assert.Equal("\"Hello world\"", value);
    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[Fact]
public async Task Should_SampleFunctionTool_Static_StringAsync()
{
var tools = new Tools([MethodFunctionTools.SampleFunctionTool_Static_StringAsync]);
var value = await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_Static_StringAsync), "{\"input\":\"test\"}");
Assert.Equal(value, "\"Hello world\"");
}
[Fact]
public async Task Should_SampleFunctionTool_Static_StringAsync()
{
var tools = new Tools([MethodFunctionTools.SampleFunctionTool_Static_StringAsync]);
var value = await tools.CallAsync(nameof(MethodFunctionTools.SampleFunctionTool_Static_StringAsync), "{\"input\":\"test\"}");
Assert.Equal("\"Hello world\"", value);
}

@HavenDV HavenDV merged commit fe135d9 into tryAGI:main Mar 10, 2025
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants